Skip to content

[core][common]support row range read of a DataSplit - #9705

Open
weijietong wants to merge 5 commits into
apache:masterfrom
weijietong:range_read_1
Open

[core][common]support row range read of a DataSplit#9705
weijietong wants to merge 5 commits into
apache:masterfrom
weijietong:range_read_1

Conversation

@weijietong

Copy link
Copy Markdown
Contributor

Purpose

This is to solve issue.

Tests

tests for pk table,append table, DE table.

@JingsongLi

Copy link
Copy Markdown
Contributor

Why not just use IndexedSplit?

@weijietong

Copy link
Copy Markdown
Contributor Author

The critical RowRange use case is slicing the stream after a filter — e.g. "give me the 1000th–2000th matching rows for training." Even composed with a filter, IndexedSplit only ANDs two physical-position bitmaps; it never renumbers rows into a
post-filter effective sequence, so it cannot express "the N-th row after filtering."

This is exactly what AbstractDataTableRead.outerWrap encodes:

  • RowRange + filter (executeFilter): outerWrap=true — RowRange wraps outside the filter, slicing the filtered effective stream.
  • IndexedSplit + filter: both reduce to physical-position bitmaps ANDed together — no "post-filter renumbering" step.

The AI training-shard use case (why RowRange exists)

In AI / ML data loading, a common pattern is sharding a dataset by effective sample position:

  • A training job with worldSize=W, rank=R wants its local shard — the rows at effective positions [R*N, (R+1)*N - 1] of the dataset.
  • With filters active (e.g. quality filters, column predicates) or deletion vectors, the "effective row order" is the filtered/compacted sequence. Each worker must receive a deterministic, contiguous slice of that sequence, not of the raw file layout.
  • RowRange models exactly this: a single contiguous [start, end] in the 0-based effective-row space of a split. The reader either pushes it down as a selection bitmap (parquet, full-scan — no extra rows decoded) or wraps a single RangeSkipReader over
    the filtered output (filter/DV/ORC/merge-tree) to skip start and take count.

IndexedSplit cannot express this:

  • It cuts physical row-ids, which have no relation to the post-filter effective order — worker rank=R wouldn't get a contiguous slice of matching rows.
  • It requires firstRowId (so plain append tables are out) or a pre-built global index.
  • For merge-tree primary-key tables, physical position ≠ output order, so the shard would be scrambled.

Conclusion

IndexedSplit and RowRange are complementary, not interchangeable:

  • IndexedSplit: read a discrete set of physical row-ids (vector search / global-index hits), AND-composed with filters in the physical-position space, requires row-ids or a pre-built index, excludes append tables / post-filter slicing / merge-output
    slicing.
  • RowRange: read a contiguous slice in effective-row space (AI training shards / pagination / range queries), applied as skip+limit over the filtered effective stream, works across append / primary-key / DE tables, no pre-built index needed.

RowRange was introduced precisely to cover IndexedSplit's blind spots — append tables, post-filter effective-row slicing, and merge-output slicing — all validated by tests in this PR (testAppendOrcRowRange..., testRowRangeWithFilterWrapsOutsideFilter,
testPrimaryKeyRowRangeOverMergeRead). They cover behavior IndexedSplit cannot express, so RowRange cannot be replaced by it.

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The post-filter/merge-output slicing use case is distinct from IndexedSplit's physical row IDs and has end-to-end value. Two fallback paths currently violate the requested range; both were reproduced using real table writes and reads, and both passed after focused conditional fixes. Details are inline.

The existing Parquet DE range test also passes. The new failures specifically cover ORC column-merge reads and a supported file-format change within an append table.

fileIndexResult =
FileIndexEvaluator.evaluate(
fileIO,
formatReaderMapping.getDataSchema(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Avoid applying the effective range twice for ORC column merges

For a DE table with multiple column files, createFieldBunchReader(DataBunch) reaches this overload with fileRowRange set and fileIndexResult null. This block creates a range bitmap even when the format cannot push it down. ApplyBitmapIndexRecordReader then applies that bitmap, and the ORC fallback at lines 785–786 skips the range start again. A real 100-row DE table stored as two ORC column files returns [] for RowRange.of(10, 19), while the ordinary reader returns 0..99 and the expected slice is 10..19.

Construct this bitmap only when that format can use the range path, or explicitly track that the range has already been applied before wrapping RangeSkipReader. Add a partial-column ORC read regression; the existing Parquet case does not cover this branch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

rowRange != null && !hasFilter && topN == null && limit == null && !hasDv;
boolean canPushdown =
fullScanRange
&& !files.isEmpty()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Check every file before disabling the split-level range fallback

canPushdown checks only the first file. After an append table changes from Parquet to ORC, one split can contain both formats. If Parquet comes first, the outer RangeSkipReader is disabled, while createFileReader for the ORC file neither pushes down nor applies its local range. I reproduced this with Parquet rows 0..4 followed by ORC rows 5..9 in one split: range [3,6] returns [3,4,5,6,7,8,9], instead of [3,4,5,6]. This leaks rows from the next training shard. Require all files to support pushdown, or enforce each unsupported file's local slice. The regression passes when this capability check covers every file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants